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

Link peer's X509 stack handle to parent SSL safe handle #113124

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

rzikm
Copy link
Member

@rzikm rzikm commented Mar 4, 2025

Fixes #109689.

Fixes rare data race between handshake and SslStream.Dispose() where we would try to access already freed STACKOF(X509) and crash.

I also reviewed other interop usages to make sure there are no similar bugs lurking elsewhere on SslStream codebase.

Tested locally by inserting artificial delays at relevant places.

@Copilot Copilot bot review requested due to automatic review settings March 4, 2025 13:30

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR addresses a race condition during SSL/TLS handshakes by restructuring how the peer certificate chain is retrieved.

  • Renames the original SslGetPeerCertChain method to a private implementation (SslGetPeerCertChain_private).
  • Introduces a new public wrapper for SslGetPeerCertChain that uses SafeInteriorHandle.OpenInteriorHandle.

Reviewed Changes

File Description
src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs Refactors peer certificate retrieval to link the parent SSL safe handle using a new wrapper method

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.Ssl.cs:126

  • [nitpick] Consider renaming 'SslGetPeerCertChain_private' to a more conventional name such as 'SslGetPeerCertChainCore' to clarify its role as the internal implementation detail.
internal static partial SafeSharedX509StackHandle SslGetPeerCertChain_private(SafeSslHandle ssl);
…aphy.Native/Interop.Ssl.cs

Co-authored-by: Kevin Jones <vcsjones@github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dotnet OpenSSL double free with Ubuntu 24.04
2 participants