Skip to content

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
@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