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

ssl: refactor check_supported_protocol_versions #866

Merged
merged 1 commit into from
Feb 27, 2025

Conversation

rhenium
Copy link
Member

@rhenium rhenium commented Feb 27, 2025

As reported in ruby/ruby#12823, check_supported_protocol_versions is unstable and occasionally fails with Errno::ECONNABORTED during SSLSocket#connect on Windows.

When the server-side SSLContext specifies an unsupported SSL/TLS protocol version, start_server accepts a TCP connection but closes it without reading ClientHello, as SSLSocket#accept immediately raises an exception. With Winsock, this can cause the client-side SSLSocket#connect to raise Errno::ECONNABORTED.

While the simplest fix is to add rescue Errno::ECONNABORTED, this method can be simplified. Instead, let's set up a server that accepts all protocol versions and test client connections with different settings.

As reported in <ruby/ruby#12823>,
check_supported_protocol_versions is unstable and occasionally fails
with Errno::ECONNABORTED during SSLSocket#connect on Windows.

When the server-side SSLContext specifies an unsupported SSL/TLS
protocol version, start_server accepts a TCP connection but closes it
without reading ClientHello, as SSLSocket#accept immediately raises an
exception. With Winsock, this can cause the client-side
SSLSocket#connect to raise Errno::ECONNABORTED.

While the simplest fix is to add rescue Errno::ECONNABORTED, this method
can be simplified. Instead, let's set up a server that accepts all
protocol versions and test client connections with different settings.
@rhenium rhenium merged commit 8c49897 into ruby:master Feb 27, 2025
54 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

1 participant