Skip to content

Fix GH-10495: feof on OpenSSL stream hangs indefinitely #13487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

bukka
Copy link
Member

@bukka bukka commented Feb 24, 2024

This fixes the issue with unbounded waiting on SSL_peek which can happen when only part of the record is fetched. It makes socket non blocking so it is possible to verify if OpenSSL is expecting some more data or if there is an error.

It should be noted that most of the changes are for handling situation where timeout is requested. This is however not used by the only two calls using the PHP_STREAM_OPTION_CHECK_LIVENESS option (php_stream_eof and php_stream_xport_create). Both of them set 0 as value which means no waiting should happen. The case where some value is set and timeout should be used might be only used by some external extension but it is quite unlikely as those would most likely use higher level function and most of the time just check eof. But it is done to make the logic complete and at least somehow consistent. We should however change it in master so I will create a ticket for this.

@bukka
Copy link
Member Author

bukka commented Feb 25, 2024

MacOS error related - will hopefully get it sorted out next week.

@bukka
Copy link
Member Author

bukka commented Mar 10, 2024

I have been debugging this on MacOS and looks like that the test is not completely correct because one TCP frame can contain multiple TLS records which is happening on MacOS much more often than on Linux (haven't actually seen this on Linux but it's also theoretically possible). So I will need to apply a bit smarter logic that gets length and reads multiple records. Will dig into more in the next few weeks as I already spent a bit of time on debugging and need to also do other things.

This fixes the issue with unbounded waiting on SSL_peek which can happen
when only part of the record is fetched. It makes socket non blocking so
it is possible to verify if OpenSSL is expecting some more data or if
there is an error.
@bukka bukka force-pushed the openssl_liveness_check_freeze branch from ed1c417 to d4a2a73 Compare March 23, 2024 14:38
@bukka bukka closed this in c1bd9a9 Mar 29, 2024
@bukka
Copy link
Member Author

bukka commented Mar 29, 2024

Accidentally merged the old version (did the test fix on my Mac and forgot to pull...) so follow up test fix is here: 100258f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant