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

Fix: OpenSSL 3.x reports unexpected EOF as SSL error #14219

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Jan 11, 2024

Prior versions used to report a syscall error with a zero return code but this has been considered a bug and changed to be reported as a SSL error with a specific reason in v3.

This patch is implementing the detection of an unexpected EOF for v3 while still supporting v1 as per the documentation:

But it doesn't fix the failing spec, which only seem to have started failing with OpenSSL v3.2 while the change was introduced in OpenSSL v3.0 🤔

Details:

  • return code is -1;
  • error type is SSL_ERROR_SYSCALL;
  • error code is 0;
  • error message is "unknown error or no error".

Refs #14200 #14168 #14169

Prior versions used to report a syscall error with a zero return code
but this has been considered a bug and changed to be reported as a SSL
error with a specific reason in v3.
@ysbaddaden ysbaddaden changed the title Fix: OpenSSL 3.x reports unexpected error as SSL error Fix: OpenSSL 3.x reports unexpected EOF as SSL error Jan 11, 2024
This was linked to issues Jan 11, 2024
@Blacksmoke16 Blacksmoke16 added kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking labels Jan 11, 2024
@ysbaddaden
Copy link
Contributor Author

I thought maybe the issue was in how we report EOF in the BIO integration. I tried to modify bread_ex to return 0 when we reach EOF (read returned zero) but it has no effect.

I debug the bread_ex method and we indeed reach EOF (read returned zero) before we get the SSL error. The problem is that it's not reported as it's supposed to be: return code is -1 (expected) but the error type should now be SSL but it's SYSCALL.

@ysbaddaden ysbaddaden marked this pull request as ready for review February 26, 2024 10:53
@ysbaddaden
Copy link
Contributor Author

I don't think I can do better. I'm really not sure about the hack to fix the spec.

I guess we should check with 2 processes talking through SSL and killing either one and see if the EOF is correctly detected. If someone's willing to check that...

@straight-shoota
Copy link
Member

straight-shoota commented Mar 13, 2024

I tried this patch with a simple SSL client/server communication using OpenSSL 3.2.1. Both sides detect EOF correctly if the other end disappears.
So looks like everything works as expected.

@straight-shoota straight-shoota added this to the 1.12.0 milestone Mar 13, 2024
@straight-shoota straight-shoota merged commit eb743de into crystal-lang:master Mar 15, 2024
57 checks passed
@ysbaddaden ysbaddaden deleted the fix/openssl-detect-unexpected-eof branch March 15, 2024 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

make std_spec bug Support for OpenSSL 3.2.0
3 participants