Skip to content
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

TestPumaControlCli#test_control_ssl test failure testing against OpenSSL 3.x #2753

Closed
voxik opened this issue Nov 15, 2021 · 12 comments
Closed

Comments

@voxik
Copy link
Contributor

voxik commented Nov 15, 2021

I am testing Ruby OpenSSL 3.x support and I observe following test failure:

   7) Failure:
TestPumaControlCli#test_control_ssl
[/builddir/build/BUILD/puma-4.3.6/usr/share/gems/gems/puma-4.3.6/test/test_pumactl.rb:170]:
Expected /Command\ stop\ sent\ success/ to match "SSL_read: unexpected
eof while reading\n".

It seems that this is caused by OpenSSL changing its behavior in EOF handling in SSL_read and this is the affected code:

https://github.com/ruby/openssl/blob/8193b7322ece2548eaf3d8acd1aec32bfc2cfdb9/ext/openssl/ossl_ssl.c#L1834-L1835

However, the question is if this is the right behavior. Please see the ruby/openssl#399 (comment) for more details.

@MSP-Greg
Copy link
Member

@voxik

Sorry for the delay. I think this is the same breaking change that was introduced in 1.1.1e and reverted in 1.1.1f.

Was the failure intermittent? I haven't checked Windows, but with WSL2/Ubuntu 20.04 using Ruby master and OpenSSL 3.0.1, I can't get it to fail? Somewhere I've got a patch that passes the OpenSSL error info/number up into Puma...

@voxik
Copy link
Contributor Author

voxik commented Jan 19, 2022

Hmmm, I don't remember anymore if the failure was intermittent or not, but I'd say it was reproducible. I have workaround the issue by this patch for ruby/openssl to restore the original Ruby behavior:

@dentarg
Copy link
Member

dentarg commented Jan 19, 2022

Looks like you are testing Puma 4.3.6 @voxik, is that correct? I have a hard time seeing Puma 4.x being updated for OpenSSL 3.x

@voxik
Copy link
Contributor Author

voxik commented Jan 19, 2022

Yes, that is correct. That is the version which is in Fedora. There is pending update to the most recent version, so that should be no problem.

@dentarg
Copy link
Member

dentarg commented Jan 19, 2022

Can we close? We can re-open if there are still problems with the Puma version that has been updated for OpenSSL 3.

@MSP-Greg
Copy link
Member

Looks like you are testing Puma 4.3.6

Thanks @dentarg. Sometimes I'm very good at missing the obvious...

@MSP-Greg
Copy link
Member

MSP-Greg commented Jan 19, 2022

@voxik Any idea when Fedora hopes to begin using OpenSSL 3? Or is it too early?

@voxik
Copy link
Contributor Author

voxik commented Jan 20, 2022

Fedora Rawhide is using OpenSSL 3 since end of September. That is the reason I had to backport the patches into Ruby 3.0 and therefore I identified this issue. Is there any reason to believe that Puma has changed to handle the EOF differently?

@voxik
Copy link
Contributor Author

voxik commented Jan 20, 2022

Just briefly checking CI, I can't see OpenSSL 3 to be tested anywhere, but I might be missing something. Anyway, I can give a try to Puma 5.x against Ruby 3.1 without the EOF patch and can report back.

@dentarg
Copy link
Member

dentarg commented Jan 20, 2022

Is there any reason to believe that Puma has changed to handle the EOF differently?

From #2753 (comment)

with WSL2/Ubuntu 20.04 using Ruby master and OpenSSL 3.0.1, I can't get it to fail

Of course, that might not be perfect equivalent to running on Fedora

Anyway, I can give a try to Puma 5.x against Ruby 3.1 without the EOF patch and can report back.

That would be great

@MSP-Greg
Copy link
Member

@voxik

Just briefly checking CI, I can't see OpenSSL 3 to be tested anywhere

Correct. Not in our CI. I've wondered about adding one or two OpenSSL 3 builds to GitHub Actions.

Is there any reason to believe that Puma has changed to handle the EOF differently?

I believe the actual read code hasn't changed, but things have changed with both the server writes and the write in control_cli.rb.

Thanks for working with Puma in Fedora. Since you're probably used to working with patches, if you can check against PR #2800, that would be very helpful.

@voxik
Copy link
Contributor Author

voxik commented Jan 20, 2022

@voxik

Just briefly checking CI, I can't see OpenSSL 3 to be tested anywhere

Correct. Not in our CI. I've wondered about adding one or two OpenSSL 3 builds to GitHub Actions.

👍

Is there any reason to believe that Puma has changed to handle the EOF differently?

I believe the actual read code hasn't changed, but things have changed with both the server writes and the write in control_cli.rb.

Thx for explanation. Now trying puma 5.2.2 with following configuration:

$ rpm -q ruby
ruby-3.1.0-160.fc36.x86_64

$ rpm -q openssl-libs
openssl-libs-3.0.0-1.fc36.x86_64

I can't reproduce the issue (unless I made some mistake). So I'm going to drop the EOF patch from Ruby and hope for the best.

Thanks for working with Puma in Fedora. Since you're probably used to working with patches, if you can check against PR #2800, that would be very helpful.

I'll take a look

@voxik voxik closed this as completed Jan 20, 2022
liuyangxy pushed a commit to fedora-riscv/ruby that referenced this issue Jan 26, 2023
This patch was originally added due to Puma, but it seems it is not
needed anymore:

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

No branches or pull requests

4 participants