Skip to content

🥅 Re-raise #starttls error from receiver thread - #395

Merged
nevans merged 1 commit into
masterfrom
fix-starttls-swallowed-exception
Feb 4, 2025
Merged

🥅 Re-raise #starttls error from receiver thread#395
nevans merged 1 commit into
masterfrom
fix-starttls-swallowed-exception

Conversation

@nevans

@nevans nevans commented Feb 3, 2025

Copy link
Copy Markdown
Collaborator

Fixes #394.

When start_tls_session raises an exception, that's caught in the receiver thread, but not re-raised. Fortunately, @sock will now be a permanently broken SSLSocket, so I don't think this can lead to accidentally using an insecure connection.

Even so, #starttls should disconnect the socket and re-raise the error immediately.

Failing test case was provided by @rhenium in #394.

Fixes #394.

When `start_tls_session` raises an exception, that's caught in the
receiver thread, but not re-raised.  Fortunately, `@sock` will now be
a permanently broken SSLSocket, so I don't think this can lead to
accidentally using an insecure connection.

Even so, `#starttls` should disconnect the socket and re-raise the error
immediately.

Failing test case was provided by @rhenium in #394.

Co-authored-by: Kazuki Yamaguchi <k@rhe.jp>
@nevans
nevans requested a review from rhenium February 4, 2025 00:03
@nevans
nevans merged commit 257ede0 into master Feb 4, 2025
@nevans
nevans deleted the fix-starttls-swallowed-exception branch February 4, 2025 15:11
@nevans nevans added the bug Something isn't working label Feb 4, 2025
nevans added a commit that referenced this pull request Apr 23, 2026
…ckports #395]

Backports #395 to `v0.3-stable`.  The tests required an additional
rescue-and-ignore for the server thread in `starttls_test`, which was
already present in all later branches.

---------

When `start_tls_session` raises an exception, that's caught in the
receiver thread, but not re-raised.  Fortunately, `@sock` will now be
a permanently broken SSLSocket, so I don't think this can lead to
accidentally using an insecure connection.

Even so, `#starttls` should disconnect the socket and re-raise the error
immediately.

Failing test case was provided by @rhenium in #394.

Co-authored-by: Kazuki Yamaguchi <k@rhe.jp>
nevans added a commit that referenced this pull request Apr 23, 2026
🔒 Fix STARTTLS stripping vulnerability (backports #664, #395, #198)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

1 participant