Skip to content

Conversation

MackinnonBuck
Copy link
Member

@MackinnonBuck MackinnonBuck commented Jul 29, 2025

Backport of #62531 to release/8.0

Dispose the certificate chain elements with the chain

Fixes an issue in certificate authentication where certificates within a certificate chain were not getting directly disposed.

Description

Failing to dispose each certificate within an X509 chain can create significant GC pressure for applications that frequently perform TLS handshakes. While the previous disposal logic only disposed the X509Chain itself, this PR updates the logic to first enumerate and dispose each certificate in the chain directly.

Customer Impact

The original contribution was from a customer who determined that this issue has a severe negative performance impact on their large scale web application. See #62531 (comment).

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

The change is straightforward and follows an established disposal pattern.

Verification

  • Manual (required)
  • Automated

Packaging changes reviewed?

  • Yes
  • No
  • N/A

* Dispose the certificate chain elements with the chain

* Fix the missing brace

* Remove snarky comment.

* Add another choice using based on review feedback

* Styling fixes

---------

Co-authored-by: Mackinnon Buck <[email protected]>
@Copilot Copilot AI review requested due to automatic review settings July 29, 2025 20:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses a resource management issue by ensuring that X509Certificate2 objects in certificate chains are properly disposed. The fix prevents potential memory leaks by manually disposing all certificate chain elements before disposing the chain itself.

Key changes:

  • Replace using declarations with try-finally blocks for X509Chain disposal
  • Add manual disposal of certificate chain elements before disposing the chain
  • Fix a syntax error (semicolon to closing brace)

Reviewed Changes

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

File Description
src/Shared/CertificateGeneration/UnixCertificateManager.cs Adds proper disposal pattern for X509Chain and its elements, fixes syntax error
src/Security/Authentication/Certificate/src/CertificateAuthenticationHandler.cs Implements the same disposal pattern for certificate validation
Comments suppressed due to low confidence (1)

src/Shared/CertificateGeneration/UnixCertificateManager.cs:178

  • This appears to be fixing a syntax error where a semicolon was incorrectly used instead of a closing brace.
        }

@dotnet-policy-service dotnet-policy-service bot added this to the 8.0.x milestone Jul 29, 2025
@MackinnonBuck MackinnonBuck added the Servicing-consider Shiproom approval is required for the issue label Jul 29, 2025
@MackinnonBuck MackinnonBuck added Servicing-approved Shiproom has approved the issue and removed Servicing-consider Shiproom approval is required for the issue labels Jul 30, 2025
@wtgodbe wtgodbe merged commit a6efb8b into release/8.0 Aug 5, 2025
23 of 25 checks passed
@wtgodbe wtgodbe deleted the mbuck/backport-cert-fix-to-8.0 branch August 5, 2025 01:05
@dotnet-policy-service dotnet-policy-service bot modified the milestones: 8.0.x, 8.0.20 Aug 5, 2025
This was referenced Sep 9, 2025
This was referenced Sep 15, 2025
michael-j-green pushed a commit to gaseous-project/gaseous-server that referenced this pull request Sep 17, 2025
Updated
[Microsoft.AspNetCore.OpenApi](https://github.com/dotnet/aspnetcore)
from 8.0.19 to 8.0.20.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.OpenApi's
releases](https://github.com/dotnet/aspnetcore/releases)._

## 8.0.20

[Release](https://github.com/dotnet/core/releases/tag/v8.0.20)

## What's Changed
* Update branding to 8.0.20 by @​vseanreesermsft in
dotnet/aspnetcore#63106
* [release/8.0] (deps): Bump src/submodules/googletest from `c67de11` to
`373af2e` by @​dependabot[bot] in
dotnet/aspnetcore#63038
* [release/8.0] Dispose the certificate chain elements with the chain by
@​MackinnonBuck in dotnet/aspnetcore#62994
* [release/8.0] Update SignalR Redis tests to use internal Docker Hub
mirror by @​github-actions[bot] in
dotnet/aspnetcore#63117
* [release/8.0] [SignalR] Don't throw for message headers in Java client
by @​github-actions[bot] in
dotnet/aspnetcore#62784
* Merging internal commits for release/8.0 by @​vseanreesermsft in
dotnet/aspnetcore#63152
* [release/8.0] Update dependencies from dotnet/extensions by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63188
* [release/8.0] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63189


**Full Changelog**:
dotnet/aspnetcore@v8.0.19...v8.0.20

Commits viewable in [compare
view](https://github.com/dotnet/aspnetcore/commits/v8.0.20).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Microsoft.AspNetCore.OpenApi&package-manager=nuget&previous-version=8.0.19&new-version=8.0.20)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
michael-j-green added a commit to gaseous-project/gaseous-server that referenced this pull request Sep 17, 2025
… 8.0.20 (#601)

Updated
[Microsoft.AspNetCore.Authentication.OpenIdConnect](https://github.com/dotnet/aspnetcore)
from 8.0.19 to 8.0.20.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Authentication.OpenIdConnect's
releases](https://github.com/dotnet/aspnetcore/releases)._

## 8.0.20

[Release](https://github.com/dotnet/core/releases/tag/v8.0.20)

## What's Changed
* Update branding to 8.0.20 by @​vseanreesermsft in
dotnet/aspnetcore#63106
* [release/8.0] (deps): Bump src/submodules/googletest from `c67de11` to
`373af2e` by @​dependabot[bot] in
dotnet/aspnetcore#63038
* [release/8.0] Dispose the certificate chain elements with the chain by
@​MackinnonBuck in dotnet/aspnetcore#62994
* [release/8.0] Update SignalR Redis tests to use internal Docker Hub
mirror by @​github-actions[bot] in
dotnet/aspnetcore#63117
* [release/8.0] [SignalR] Don't throw for message headers in Java client
by @​github-actions[bot] in
dotnet/aspnetcore#62784
* Merging internal commits for release/8.0 by @​vseanreesermsft in
dotnet/aspnetcore#63152
* [release/8.0] Update dependencies from dotnet/extensions by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63188
* [release/8.0] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63189


**Full Changelog**:
dotnet/aspnetcore@v8.0.19...v8.0.20

Commits viewable in [compare
view](https://github.com/dotnet/aspnetcore/commits/v8.0.20).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Microsoft.AspNetCore.Authentication.OpenIdConnect&package-manager=nuget&previous-version=8.0.19&new-version=8.0.20)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Michael Green <[email protected]>
michael-j-green pushed a commit to gaseous-project/gaseous-server that referenced this pull request Sep 17, 2025
… to 8.0.20 (#600)

Updated
[Microsoft.AspNetCore.Authentication.MicrosoftAccount](https://github.com/dotnet/aspnetcore)
from 8.0.19 to 8.0.20.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Authentication.MicrosoftAccount's
releases](https://github.com/dotnet/aspnetcore/releases)._

## 8.0.20

[Release](https://github.com/dotnet/core/releases/tag/v8.0.20)

## What's Changed
* Update branding to 8.0.20 by @​vseanreesermsft in
dotnet/aspnetcore#63106
* [release/8.0] (deps): Bump src/submodules/googletest from `c67de11` to
`373af2e` by @​dependabot[bot] in
dotnet/aspnetcore#63038
* [release/8.0] Dispose the certificate chain elements with the chain by
@​MackinnonBuck in dotnet/aspnetcore#62994
* [release/8.0] Update SignalR Redis tests to use internal Docker Hub
mirror by @​github-actions[bot] in
dotnet/aspnetcore#63117
* [release/8.0] [SignalR] Don't throw for message headers in Java client
by @​github-actions[bot] in
dotnet/aspnetcore#62784
* Merging internal commits for release/8.0 by @​vseanreesermsft in
dotnet/aspnetcore#63152
* [release/8.0] Update dependencies from dotnet/extensions by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63188
* [release/8.0] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63189


**Full Changelog**:
dotnet/aspnetcore@v8.0.19...v8.0.20

Commits viewable in [compare
view](https://github.com/dotnet/aspnetcore/commits/v8.0.20).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Microsoft.AspNetCore.Authentication.MicrosoftAccount&package-manager=nuget&previous-version=8.0.19&new-version=8.0.20)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
michael-j-green pushed a commit to gaseous-project/gaseous-server that referenced this pull request Sep 17, 2025
…#599)

Updated
[Microsoft.AspNetCore.Authentication.Google](https://github.com/dotnet/aspnetcore)
from 8.0.19 to 8.0.20.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.AspNetCore.Authentication.Google's
releases](https://github.com/dotnet/aspnetcore/releases)._

## 8.0.20

[Release](https://github.com/dotnet/core/releases/tag/v8.0.20)

## What's Changed
* Update branding to 8.0.20 by @​vseanreesermsft in
dotnet/aspnetcore#63106
* [release/8.0] (deps): Bump src/submodules/googletest from `c67de11` to
`373af2e` by @​dependabot[bot] in
dotnet/aspnetcore#63038
* [release/8.0] Dispose the certificate chain elements with the chain by
@​MackinnonBuck in dotnet/aspnetcore#62994
* [release/8.0] Update SignalR Redis tests to use internal Docker Hub
mirror by @​github-actions[bot] in
dotnet/aspnetcore#63117
* [release/8.0] [SignalR] Don't throw for message headers in Java client
by @​github-actions[bot] in
dotnet/aspnetcore#62784
* Merging internal commits for release/8.0 by @​vseanreesermsft in
dotnet/aspnetcore#63152
* [release/8.0] Update dependencies from dotnet/extensions by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63188
* [release/8.0] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in dotnet/aspnetcore#63189


**Full Changelog**:
dotnet/aspnetcore@v8.0.19...v8.0.20

Commits viewable in [compare
view](https://github.com/dotnet/aspnetcore/commits/v8.0.20).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Microsoft.AspNetCore.Authentication.Google&package-manager=nuget&previous-version=8.0.19&new-version=8.0.20)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot merge` will merge this PR after your CI passes on it
- `@dependabot squash and merge` will squash and merge this PR after
your CI passes on it
- `@dependabot cancel merge` will cancel a previously requested merge
and block automerging
- `@dependabot reopen` will reopen this PR if it is closed
- `@dependabot close` will close this PR and stop Dependabot recreating
it. You can achieve the same result by closing it manually
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Servicing-approved Shiproom has approved the issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants