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

[release/8.0] Removed unused sessions from SSL_CTX internal cache #102095

Open
wants to merge 3 commits into
base: release/8.0-staging
Choose a base branch
from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented May 10, 2024

Backport of #101684 to release/8.0-staging

/cc @wfurt @rzikm

Customer Impact

Reported by customer via official support. Small repro available.
Customer sees increased memory usage when establishing large amount of connections to the same host in a quick succession.

Workaround is lowering the cache size via

  • System.Net.Security.TlsCacheSize AppCtx switch
  • DOTNET_SYSTEM_NET_SECURITY_TLSCACHESIZE environment variable

The mechanism of the (bounded) memory leak is as follows:

  • on new TLS connection, we receive TLS session ticket so that future connections can use TLS Resume (for faster handshake)
  • There are 2 TLS session caches: internal one in OpenSSL, and managed one in .NET, these caches are supposed to be in sync via callbacks (e.g. OpenSSL may remove an entry internally and notifies managed code via callback that a ticket has been invalidated)
  • when many connections are opened in a burst, we receive many TLS session tickets in quick succession, but the implementation keeps only the last one
  • Sessions discarded from managed (.NET) TLS session cache were still being kept in the internal OpenSSL cache, until the ticket expired or the internal cache reached a predefined maximum size

The fix is to keep the two caches in sync and remove the dropped TLS session tickets from the internal cache as well.

Regression

Yes, the bug is part of TLS Session resumption feature on Linux, introduced in .NET 7.

Testing

Tested on customer provided repro, verified by tracking OpenSSL allocations in the app.

Risk

Low, the issue is well understood and the change is localized to the feature. Functional tests will verify TLS resumption works.

Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

@rzikm rzikm requested review from wfurt and karelz May 13, 2024 08:09
@rzikm rzikm changed the title [release/8.0-staging] Removed unused sessions from SSL_CTX internal cache [release/8.0] Removed unused sessions from SSL_CTX internal cache May 13, 2024
Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

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.

None yet

3 participants